ICU-22950 Do not initialize currency for NumberPropertyMapper when useCurrency is false #3250
Conversation
resolveCurrency() slows down simple number formatting requests like: for (int i = 0; i < 9999; i++) u_snprintf_u(buff, 100, u"%d", i); Since we don't use currency here, we can skip it's initialization.
|
|
| CurrencyUnit currency; | ||
| UCurrencyUsage currencyUsage; |
There was a problem hiding this comment.
I expanded the rest of this (long) function, searched for "currency", and found some code that uses these variables but is gated by conditions other than useCurrency.
if (!properties.currencyUsage.isNull())if (exportedProperties != nullptr)
If we think we can guarantee that useCurrency is true there, then we should add U_ASSERT(useCurrency) there.
For the second one, maybe a default-constructed currency is fine if not useCurrency?
Otherwise, we may need to add a runtime check for useCurrency.
There was a problem hiding this comment.
Markus is right; this is a concern.
For if (!properties.currencyUsage.isNull()), I wonder if this could just be changed to useCurrency?
For exportedProperties->currency, I agree that a default-constructed currency ought to be fine.
For if (precision.fType == Precision::PrecisionType::RND_CURRENCY), I wonder if we could change this to if (useCurrency && precision.fType == Precision::PrecisionType::RND_CURRENCY)?
There was a problem hiding this comment.
Added an assertion to if (!properties.currencyUsage.isNull()), since useCurrency is true when !properties.currencyUsage.isNull()
Used if (useCurrency && precision.fType == Precision::PrecisionType::RND_CURRENCY), thank you!
resolveCurrency() slows down simple number formatting requests like:
Since icu4c doesn't use currency there, we can skip its initialization.
Checklist